Integrated bare etiss processor #106
Conversation
Samanti-Das
force-pushed
the
integrated_bare_etiss_processor
branch
from
374f784 to
2f04bf8
Compare
CMakeLists.txt
Outdated
| @@ -85,10 +86,23 @@ ELSE() | |||
| SET(CMAKE_LINKER_FLAGS_DEBUG "${CMAKE_LINKER_FLAGS_DEBUG} -fsanitize=address -fsanitize=undefined") | |||
| ENDIF() | |||
|
|
|||
| # use, i.e. don't skip the full RPATH for the build tree | |||
| set(CMAKE_SKIP_BUILD_RPATH FALSE) | |||
There was a problem hiding this comment.
i don't think hacking the rpath is a great way to solve this.
i'd rather suggest that the user is responsible to set the $PATH/$LD_LIBRARY_PATH. we could add convenience by doing that in run_helper.sh
but as long as the install rpath remains unchanged (which i don't think it is), it would still be fine for me.
There was a problem hiding this comment.
As i understand it, the way things are handled currently are:
RPATH is set to $ORIGIN, which effectively enables loading of shared objects from relative directories. In the master branch, bare_etiss_processor gets built separately, and as such does not receive $ORIGIN as its RPATH, but an absolute path which points to the ETISS libraries.
Leaving the RPATH settings as-is in this PR would mean that bare_etiss_processor now also has an RPATH of $ORIGIN, which is incorrect as bare_etiss_processor is not placed in the same directory as the libraries.
Two options come to mind:
- moving the
RPATH-related changes from the top-levelCMakeLists.txttosrc/bare_etiss_processor/CMakeLists.txtand restoring the top-levelCMakeLists.txt - restoring the top-level
CMakeLists.txtand adding a relativeRPATHentry tobare_etiss_processor, pointing to$ORIGIN/../lib
There was a problem hiding this comment.
I would say changing RPATH settings on a global scale via CMAKE_* variables is not the best case.
You can also set RPATH stuff as a single target properties, e.g.:
https://cmake.org/cmake/help/latest/prop_tgt/BUILD_RPATH.html
https://cmake.org/cmake/help/latest/prop_tgt/SKIP_BUILD_RPATH.html
and so on...
There was a problem hiding this comment.
alright then i'd suggest to keep this code location unchanged to retain the homogenous cross platform behavior and move the changes to the bare_etiss target only. ideally this would also keep the $ORIGIN entry. whether that happens through cmake scopes or target properties does not matter to me.
|
Looks good otherwise, thanks! Please make sure not to drop the changes from the conflict: #102 |
This pull request moves
bare_etiss_processorfrom the examples tree into the main ETISS directory structure, and also enables it to be run directly from the build directory without the need to install ETISS first. The functionality remains unchanged,bare_etiss_processorand its companionrun_helper.shnow get compiled to<PREFIX>/bin, where<PREFIX>is either the build or the install directory.A few hacks had to be done to enable proper execution from the build tree, most notably creating and populating a directory structure for the various include files for jitlibs and ETISS defines.